Skip to content

docs(handler-authoring): expand with salesagent migration production patterns#326

Merged
bokelley merged 3 commits intomainfrom
claude/issue-229-handler-authoring-advanced-patterns
Apr 30, 2026
Merged

docs(handler-authoring): expand with salesagent migration production patterns#326
bokelley merged 3 commits intomainfrom
claude/issue-229-handler-authoring-advanced-patterns

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #229

Summary

Expands docs/handler-authoring.md (838 → 1075 lines) with six advanced production patterns from the salesagent MCP migration, and adds a cross-reference in examples/mcp_with_auth_middleware.py. Also fixes a pre-existing bug in the Error handling section (raise adcp_error(...)return adcp_error(...); adcp_error returns a dict[str, Any], so raising it would TypeError at runtime).

New / expanded sections:

  • ResolvedIdentity with DB enrichment — full second DB hop example, return None pattern so handlers convert failures to adcp_error("AUTH_REQUIRED") instead of raising an opaque 500
  • Pattern 2b — tenant routing via subdomain — two-middleware shape (SubdomainTenantMiddleware extracts tenant into ContextVar; validate_token cross-checks) with cross-tenant replay guard
  • validate_discovery_set() usage — new paragraph showing extension guard + corrected inline comment (unknown names AND mutating tools)
  • @store.wrap idempotency wiring — decorator example, PgBackend recommendation, caller_identity + tenant_id prerequisite explanation
  • Error handling rewritereturn adcp_error(...) fix, error-code taxonomy table (transient/correctable/terminal), adcp_error vs ADCPTaskError client/server distinction
  • Troubleshooting section — 5 symptom → cause → fix entries (idempotency not firing, context_factory dict error, empty tools/list, validate_discovery_set ValueError, AuthenticationRequired → 500)

What tested

  • ruff check examples/mcp_with_auth_middleware.py — passed
  • Docs-only change: no pytest/mypy run required per the pre-PR build gate
  • All new section headings verified present via content check script; raise adcp_error pattern confirmed absent

Pre-PR review

  • code-reviewer: approved — raise→return fix confirmed correct (adcp_error is -> dict[str, Any]); ContextVar reset pattern sound; @idempotency.wrap matches store API; ADCPTaskError client/server distinction accurate. 1 nit noted below.
  • docs-expert: approved after blocker fix — raise/return contradiction resolved; cross-reference to Troubleshooting anchor works; return None + caller-check pattern teaches adopters correctly. 1 nit noted below.

Nits (not fixed — surface for reviewer):

  • code-reviewer: host.count(".") >= 2 in SubdomainTenantMiddleware also passes two-label subdomains (ads.example.comads). Add a comment if three-label-only was intended; otherwise the current behavior is fine for domains with N ≥ 2 dots.
  • docs-expert: The pre-existing _resolve_identity skeleton at lines 82–90 (in the _impl pattern section, predating this PR) still shows raise AuthenticationRequired(). It now diverges from the corrected example below it. Consider aligning in a follow-up.

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_01FxyYMBreWYeJxcJQCqsBbn


Generated by Claude Code

claude added 2 commits April 30, 2026 14:49
…patterns

Closes #229

Adds six advanced production patterns that surfaced from the salesagent
MCP migration, expanding the existing guide from 838 to 1061 lines:

- ResolvedIdentity DB-enrichment flow (second DB hop beyond auth)
- Pattern 2b: subdomain tenant routing with two-middleware shape
- validate_discovery_set() usage for discovery extension guard
- @store.wrap idempotency wiring with factory prerequisites
- Error handling section fixed (return not raise) + ADCPTaskError
  client/server distinction + error-code taxonomy table
- Troubleshooting section (5 symptom → cause → fix entries)

Also adds back-reference to advanced patterns in
examples/mcp_with_auth_middleware.py docstring.

https://claude.ai/code/session_01FxyYMBreWYeJxcJQCqsBbn
…discovery_set comment

- _resolve_identity example now returns None on failure so handlers
  can convert to adcp_error("AUTH_REQUIRED") — eliminates contradiction
  with the Troubleshooting section
- validate_discovery_set inline comment updated to mention it also
  rejects mutating tools (not only unknown names)

https://claude.ai/code/session_01FxyYMBreWYeJxcJQCqsBbn
Three small fixes on top of triage's PR #326:

1. Stale _impl skeleton at line 84 still showed
   raise AuthenticationRequired(), which contradicts the new
   return-None pattern documented immediately below. A first-time
   reader copying that line would hit the exact 500 failure mode the
   new section warns against. Replace with the return-None shape and
   add a one-line callout pointing at the Error handling section.

2. PgBackend import-path missing. Wiring example imported
   MemoryBackend from adcp.server then said 'swap MemoryBackend for
   PgBackend' without showing PgBackend's actual import. PgBackend
   lives in adcp.server.idempotency, not the top-level adcp.server
   namespace; agent code-gen would guess wrong. Add the explicit
   import line.

3. Cross-reference inflation. README footer claimed
   examples/mcp_with_auth_middleware.py demonstrated 'the tenant-
   routing middleware pattern from Pattern 2b' — it doesn't, the
   example only wires BearerTokenAuthMiddleware. Soften to
   'foundation for Pattern 2b; bring your own subdomain-routing
   middleware on top.'

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley marked this pull request as ready for review April 30, 2026 15:00
@bokelley bokelley merged commit ce4c5df into main Apr 30, 2026
9 of 11 checks passed
@bokelley bokelley deleted the claude/issue-229-handler-authoring-advanced-patterns branch April 30, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-triaged documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Phase 2: Expand docs/handler-authoring.md with salesagent-migration lessons (PR-T)

2 participants